Skip to content

Release v0.6.0 - #43

Merged
hitalin merged 4 commits into
mainfrom
develop
Jul 22, 2026
Merged

Release v0.6.0#43
hitalin merged 4 commits into
mainfrom
develop

Conversation

@hitalin

@hitalin hitalin commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

変更

  • feat: server_info SWR キャッシュを導入し servers を server_detections に置換 (notedeck#782 / feat: server_info SWR キャッシュ (servers → server_detections) #42)
    • ServerInfoService — SWR (fresh 即返し / stale 返却+背景再検出 / miss は per-host dedup で検出+保存)
    • V5 migration: server_detections (生 nodeinfo + meta 保存、読取時解決) を追加し旧 servers テーブルを削除
    • StoredServer / load_servers / get_server / upsert_server を削除 (β方針)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added automatic server information detection and caching.
    • Server details now refresh in the background when cached information becomes outdated.
    • Concurrent requests avoid redundant server lookups.
    • Server metadata is retained to support more reliable detection updates.
  • Bug Fixes

    • Improved resilience when optional server metadata cannot be retrieved.
    • Updated the release version to 0.6.0.

hitalin and others added 4 commits July 22, 2026 14:25
…deck#782)

フロント (Pinia store) が持っていた「メモリ → DB → ネットワーク」の
stale-while-revalidate・TTL 判定・in-flight dedup を ServerInfoService に集約。
保存するのは生の検出結果 (nodeinfo software + /api/meta 生 JSON) で、
フォーク解決と feature 判定はアプリ側が読取時に行う — 判定ロジックの更新が
古いキャッシュに埋まる鮮度問題 (旧 servers テーブル) を構造的に解消する。

- V5 migration: server_detections 追加、旧 servers は削除 (24h キャッシュの
  ため旧データ損失は軽微)
- StoredServer / load_servers / get_server / upsert_server は削除 (β方針)
- plan_for (TTL 判定) は純関数、SWR 経路は wiremock + temp DB でテスト

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
feat: server_info SWR キャッシュ (servers → server_detections)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The release replaces the legacy server cache with raw detection storage, adds ServerInfoService with stale-while-revalidate behavior, performs concurrent nodeinfo and metadata detection, and updates persistence and tests for the new model.

Changes

Server detection cache

Layer / File(s) Summary
Raw detection model and persistence
Cargo.toml, migrations/V5__server_detections.sql, src/models.rs, src/db.rs
The package version is bumped to 0.6.0; server_detections replaces servers, and ServerDetection plus corresponding load, lookup, upsert, migration, serialization, and CRUD tests are added.
SWR detection service
src/lib.rs, src/server_info.rs
ServerInfoService classifies fresh, stale, and missing rows, deduplicates fetches, revalidates stale entries in the background, fetches nodeinfo and metadata concurrently, tolerates metadata failures, and adds integration tests. The new module is publicly exported.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant ServerInfoService
  participant Database
  participant MisskeyClient
  Caller->>ServerInfoService: get_or_fetch(host)
  ServerInfoService->>Database: get_server_detection(host)
  alt Fresh cache
    Database-->>ServerInfoService: fresh ServerDetection
    ServerInfoService-->>Caller: cached detection
  else Stale cache
    Database-->>ServerInfoService: stale ServerDetection
    ServerInfoService->>MisskeyClient: re-detect in background
    ServerInfoService-->>Caller: stale detection
  else Cache miss
    Database-->>ServerInfoService: no detection
    ServerInfoService->>MisskeyClient: fetch nodeinfo and /api/meta
    MisskeyClient-->>ServerInfoService: detection responses
    ServerInfoService->>Database: upsert_server_detection
    ServerInfoService-->>Caller: detected and stored result
  end
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the release/version bump and is clearly related to the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch develop

Comment @coderabbitai help to get the list of available commands.

@hitalin hitalin self-assigned this Jul 22, 2026
@hitalin
hitalin merged commit 96d87eb into main Jul 22, 2026
4 of 5 checks passed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
src/db.rs (2)

396-436: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract a shared row-mapper for ServerDetection.

load_server_detections and get_server_detection both inline the same 6-field closure. The codebase already has a convention for this (row_to_account); doing the same here avoids column-order drift if the schema changes.

♻️ Suggested extraction
+    fn row_to_server_detection(row: &rusqlite::Row) -> rusqlite::Result<ServerDetection> {
+        Ok(ServerDetection {
+            host: row.get(0)?,
+            software_name: row.get(1)?,
+            software_version: row.get(2)?,
+            software_repository: row.get(3)?,
+            meta_json: row.get(4)?,
+            updated_at: row.get(5)?,
+        })
+    }
+
     pub fn load_server_detections(&self) -> Result<Vec<ServerDetection>, NoteDeckError> {
         let conn = self.lock_read()?;
         let mut stmt = conn.prepare_cached(
             "SELECT host, software_name, software_version, software_repository, meta_json, updated_at FROM server_detections",
         )?;
-        let rows = stmt.query_map([], |row| {
-            Ok(ServerDetection {
-                host: row.get(0)?,
-                software_name: row.get(1)?,
-                software_version: row.get(2)?,
-                software_repository: row.get(3)?,
-                meta_json: row.get(4)?,
-                updated_at: row.get(5)?,
-            })
-        })?;
+        let rows = stmt.query_map([], Self::row_to_server_detection)?;
         Ok(rows.collect::<rusqlite::Result<Vec<_>>>()?)
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/db.rs` around lines 396 - 436, Extract the duplicated six-field
`ServerDetection` row mapping into a shared helper, following the existing
`row_to_account` convention. Update both `load_server_detections` and
`get_server_detection` to pass that helper to `query_map`, preserving their
current result and optional-row behavior.

394-436: 🚀 Performance & Scalability | 🔵 Trivial

No eviction policy for server_detections.

Unlike notes_cache/chat_messages_cache, this table has no TTL/cap-based cleanup. Rows are small, so this is likely a non-issue today, but if ServerInfoService ends up being invoked for many transient remote hosts (link previews, federated timelines), consider adding it to cleanup_with_eviction-style maintenance later.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/db.rs` around lines 394 - 436, The current server_detections additions do
not include eviction maintenance. Extend the database cleanup flow, specifically
cleanup_with_eviction or its existing maintenance path, to apply an appropriate
TTL or capacity-based eviction policy to server_detections, while preserving the
existing load_server_detections and get_server_detection behavior.
src/server_info.rs (1)

56-56: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

inflight host-lock map grows without bound.

Entries inserted by host_lock (via .entry(host).or_default()) are never removed, unlike the self-cleaning revalidating set right beside it. Over a long-running process, every distinct host that ever hits a cache miss adds a permanent String + Arc<Mutex<()>> entry that's never freed — a slow, unbounded memory leak if the service ever sees many distinct hosts (e.g. via federated content across many instances, not just logged-in accounts).

♻️ Suggested cleanup after releasing the per-host guard
             CachePlan::Miss => {
                 let lock = self.host_lock(host).await;
-                let _guard = lock.lock().await;
-                // ロック待機中に先行リクエストが保存した行を拾う (dedup)
-                if let Some(det) = self.db.get_server_detection(host)? {
-                    if plan_for(Some(&det), now_ms(), SERVER_DETECTION_TTL_MS) == CachePlan::Fresh {
-                        return Ok(det);
-                    }
-                }
-                self.detect_and_store(host).await
+                let result = {
+                    let _guard = lock.lock().await;
+                    // ロック待機中に先行リクエストが保存した行を拾う (dedup)
+                    if let Some(det) = self.db.get_server_detection(host)? {
+                        if plan_for(Some(&det), now_ms(), SERVER_DETECTION_TTL_MS)
+                            == CachePlan::Fresh
+                        {
+                            Ok(det)
+                        } else {
+                            self.detect_and_store(host).await
+                        }
+                    } else {
+                        self.detect_and_store(host).await
+                    }
+                };
+                // No other waiter holds a clone of this lock — safe to drop the entry.
+                let mut map = self.inflight.lock().await;
+                if map.get(host).is_some_and(|l| Arc::strong_count(l) == 1) {
+                    map.remove(host);
+                }
+                result
             }

Also applies to: 72-95, 145-148

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server_info.rs` at line 56, Update the inflight host-lock lifecycle in
host_lock so each host entry is removed from the map after its per-host mutex
guard is released, matching the self-cleaning behavior of revalidating. Ensure
cleanup only removes the entry associated with the completed lock and does not
race with a new waiter or lock acquisition for the same host. Keep serialization
intact while preventing stale host keys and Arc<Mutex<()>> values from
accumulating.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/server_info.rs`:
- Around line 189-197: Make the `ENV_LOCK` used by `register_insecure_host`
shared across modules, exposing or relocating it so `src/insecure.rs` uses the
same lock before mutating `NOTECLI_INSECURE_HOSTS`. Remove any module-local
duplicate lock and preserve the existing serialized environment-variable
updates.

---

Nitpick comments:
In `@src/db.rs`:
- Around line 396-436: Extract the duplicated six-field `ServerDetection` row
mapping into a shared helper, following the existing `row_to_account`
convention. Update both `load_server_detections` and `get_server_detection` to
pass that helper to `query_map`, preserving their current result and
optional-row behavior.
- Around line 394-436: The current server_detections additions do not include
eviction maintenance. Extend the database cleanup flow, specifically
cleanup_with_eviction or its existing maintenance path, to apply an appropriate
TTL or capacity-based eviction policy to server_detections, while preserving the
existing load_server_detections and get_server_detection behavior.

In `@src/server_info.rs`:
- Line 56: Update the inflight host-lock lifecycle in host_lock so each host
entry is removed from the map after its per-host mutex guard is released,
matching the self-cleaning behavior of revalidating. Ensure cleanup only removes
the entry associated with the completed lock and does not race with a new waiter
or lock acquisition for the same host. Keep serialization intact while
preventing stale host keys and Arc<Mutex<()>> values from accumulating.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4dd64c19-860d-40a4-ab2a-b44a6dc4191d

📥 Commits

Reviewing files that changed from the base of the PR and between e839286 and 36fa782.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (6)
  • Cargo.toml
  • migrations/V5__server_detections.sql
  • src/db.rs
  • src/lib.rs
  • src/models.rs
  • src/server_info.rs

Comment thread src/server_info.rs
Comment on lines +189 to +197
static ENV_LOCK: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(());

fn register_insecure_host(host: &str) {
let merged = match std::env::var("NOTECLI_INSECURE_HOSTS") {
Ok(v) if !v.is_empty() => format!("{v},{host}"),
_ => host.to_string(),
};
std::env::set_var("NOTECLI_INSECURE_HOSTS", merged);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check whether other test modules also mutate NOTECLI_INSECURE_HOSTS and whether they
# share a lock with src/server_info.rs's ENV_LOCK.
rg -n 'NOTECLI_INSECURE_HOSTS' --type=rust -C3
rg -n 'static ENV_LOCK' --type=rust -C3

Repository: notedeck-dev/notecli

Length of output: 158


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Rust files:\n'
git ls-files '*.rs' | sed -n '1,200p'

printf '\nSearch NOTECLI_INSECURE_HOSTS / ENV_LOCK / set_var occurrences:\n'
rg -n 'NOTECLI_INSECURE_HOSTS|static ENV_LOCK|set_var\(' --glob '*.rs' -C 2 || true

Repository: notedeck-dev/notecli

Length of output: 543


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Tree (top-level and src):\n'
find . -maxdepth 2 -type f | sed 's#^\./##' | sort | sed -n '1,200p'

printf '\nFocused search for NOTECLI_INSECURE_HOSTS in repository:\n'
rg -n 'NOTECLI_INSECURE_HOSTS' . || true

printf '\nFocused search for ENV_LOCK in repository:\n'
rg -n 'static ENV_LOCK|ENV_LOCK' . || true

Repository: notedeck-dev/notecli

Length of output: 1787


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'src/server_info.rs around the referenced lines:\n'
sed -n '150,240p' src/server_info.rs

printf '\nAll Rust files containing NOTECLI_INSECURE_HOSTS or ENV_LOCK:\n'
rg -n 'NOTECLI_INSECURE_HOSTS|static ENV_LOCK|Mutex::const_new|set_var\(' src --glob '*.rs' -C 3 || true

Repository: notedeck-dev/notecli

Length of output: 6236


Share the NOTECLI_INSECURE_HOSTS test lock across modules. src/insecure.rs also mutates this env var directly, so ENV_LOCK here doesn’t stop parallel tests in the same binary from racing.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server_info.rs` around lines 189 - 197, Make the `ENV_LOCK` used by
`register_insecure_host` shared across modules, exposing or relocating it so
`src/insecure.rs` uses the same lock before mutating `NOTECLI_INSECURE_HOSTS`.
Remove any module-local duplicate lock and preserve the existing serialized
environment-variable updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant